-
Notifications
You must be signed in to change notification settings - Fork 514
User: Enable roles combination - refs #5648 #6206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
$feature = api_detect_feature_context(); | ||
$permission = $feature.':edit'; | ||
|
||
return api_get_permission($permission, $userRoles); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid too many return
statements within this method.
return api_get_permission($permission, $userRoles); | ||
} | ||
|
||
return $isAllowed; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid too many return
statements within this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small change required for efficiency.
Code Climate has analyzed commit 20b053a and detected 2 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change requested for the display of user roles
public/main/admin/user_list.php
Outdated
$status_options | ||
'keyword_roles', | ||
get_lang('Roles'), | ||
array_combine(api_get_roles(), api_get_roles()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The select input should show the roles by their translated values.
The translated variables are as follow:
- ROLE_STUDENT: Learner
- ROLE_ADMIN: Admin
- ROLE_SUPER_ADMIN: Super admin
- ROLE_GLOBAL_ADMIN: Global admin
- ROLE_TEACHER: Teacher
- ROLE_HR: Human Resources Manager
- ROLE_QUESTION_MANAGER: Question manager
- ROLE_SESSION_MANAGER: Session administrator
- ROLE_STUDENT_BOSS: Superior (n+1)
- ROLE_INVITEE: Invitee
Considering we do not have such a "translations" table for roles, I would put that directly (hardcoded) into the api_get_roles() function. We will take care of that in C3.
Just a note for myself: a global admin is the person who initially installed the system (just a detail vs the super admin, which is the person managing all URLs in a multi-URL context).
@christianbeeznest aquí falta mostrar los roles en su versión traducida y corregir el conflicto de merge |
…ated role labels, multi-role search
20b053a
to
07656bf
Compare
Esta agregado las traducciones y corregido los conflictos. |
Confirmed together with @christianbeeznest that this is an elegant way to transition from legacy-permissions (only about 13 roles, most mutually-exclusive) to a new system of combination of roles that have given permissions each. |
No description provided.